Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added MarkerManager Plugin #247

Merged
merged 14 commits into from
Jul 20, 2021
Merged

Added MarkerManager Plugin #247

merged 14 commits into from
Jul 20, 2021

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Jul 1, 2021

Signed-off-by: ahcorde [email protected]

🎉 New feature

Summary

This PR is part of the consolidation between the Scene3D in ign-gui and ign-gazebo. This PR add a marker manager plugin

Test it

  • Compile the example marker inside the repo ign-gazebo
  • Add this lines to your gui.config file
<plugin filename="MarkerManager" name="Marker manager">
  <ignition-gui>
    <title>Marker Manager</title>
    <property type="bool" key="showTitleBar">false</property>
    <property type="string" key="state">docked</property>
    <property key="resizable" type="bool">false</property>
    <property key="width" type="double">5</property>
    <property key="height" type="double">5</property>
    <property key="state" type="string">floating</property>
    <property key="showTitleBar" type="bool">false</property>
  </ignition-gui>
</plugin>
  • launch gazebo ign gazebo --gui-config src/ign-gazebo/src/gui/gui.config -v 4
  • Run the marker example

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

@ahcorde ahcorde added the 🏯 fortress Ignition Fortress label Jul 1, 2021
@ahcorde ahcorde requested a review from chapulina July 1, 2021 12:08
@ahcorde ahcorde self-assigned this Jul 1, 2021
@ahcorde ahcorde requested a review from jennuine as a code owner July 1, 2021 12:08
@ahcorde ahcorde mentioned this pull request Jul 1, 2021
Merged
18 tasks
Base automatically changed from chapulina/6/new_scene to main July 1, 2021 18:23
@codecov
Copy link

codecov bot commented Jul 6, 2021 •
edited
Loading

Codecov Report

Merging #247 (cd052eb) into main (1190ed0) will increase coverage by 3.53%.
The diff coverage is 56.57%.

❗ Current head cd052eb differs from pull request most recent head 76bae05. Consider uploading reports for the commit 76bae05 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #247      +/-   ##
==========================================
+ Coverage   61.57%   65.10%   +3.53%     
==========================================
  Files          21       28       +7     
  Lines        2803     4276    +1473     
==========================================
+ Hits         1726     2784    +1058     
- Misses       1077     1492     +415
Impacted Files Coverage Δ
include/ignition/gui/GuiEvents.hh 100.00% <ø> (ø)
src/Conversions.cc 100.00% <ø> (ø)
src/plugins/image_display/ImageDisplay.cc 30.32% <0.00%> (+7.05%) ⬆️
src/Plugin.cc 57.44% <17.64%> (-2.83%) ⬇️
src/plugins/screenshot/Screenshot.cc 33.33% <33.33%> (ø)
...s/transport_scene_manager/TransportSceneManager.cc 52.29% <52.29%> (ø)
src/plugins/minimal_scene/MinimalScene.cc 53.51% <53.51%> (ø)
src/plugins/marker_manager/MarkerManager.cc 60.60% <60.60%> (ø)
src/plugins/camera_tracking/CameraTracking.cc 68.45% <68.45%> (ø)
src/plugins/scene3d/Scene3D.cc 48.89% <71.51%> (+36.11%) ⬆️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f107bd1...76bae05. Read the comment docs.

chapulina
chapulina reviewed Jul 6, 2021
View reviewed changes
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well for me 👍

How about moving the marker.cc example from ign-gazebo here? I also think we should add this plugin to scene3d.config.

src/plugins/marker_manager/MarkerManager.hh Show resolved Hide resolved
src/plugins/marker_manager/MarkerManager.cc Outdated Show resolved Hide resolved
src/plugins/marker_manager/MarkerManager.cc Outdated Show resolved Hide resolved
src/plugins/marker_manager/MarkerManager.cc Outdated Show resolved Hide resolved
src/plugins/marker_manager/MarkerManager.cc Show resolved Hide resolved
src/plugins/marker_manager/MarkerManager.cc Outdated Show resolved Hide resolved
@ahcorde
Copy link
Contributor Author

ahcorde commented Jul 7, 2021

  • moved marker.cc example
  • added plugin to scene3d.config
  • Added world statistic topic to scene provider

@ahcorde ahcorde requested a review from chapulina July 7, 2021 11:27
@ahcorde ahcorde mentioned this pull request Jul 7, 2021
Merged
5 tasks
@ahcorde ahcorde force-pushed the ahcorde/6/markermanagerplugin branch from e0d4f0b to 43bd730 Compare July 12, 2021 14:07
@ahcorde
Copy link
Contributor Author

ahcorde commented Jul 12, 2021

Added MarkerManager_TEST

chapulina
chapulina reviewed Jul 12, 2021
View reviewed changes
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the test is segfaulting

examples/standalone/marker/README.md Outdated Show resolved Hide resolved
examples/standalone/marker/README.md Outdated Show resolved Hide resolved
examples/standalone/scene_provider/scene_provider.cc Show resolved Hide resolved
examples/standalone/scene_provider/scene_provider.cc Outdated Show resolved Hide resolved
ahcorde added 12 commits July 13, 2021 09:31
@ahcorde
update MarkerManager
60fe27e 
Signed-off-by: ahcorde <[email protected]>
@ahcorde
moved markerManager test to integration
47181d8 
Signed-off-by: ahcorde <[email protected]>
@ahcorde
add more time to marker_manager_test
c53d3d3 
Signed-off-by: ahcorde <[email protected]>
@ahcorde
Enable test only on Linux
054409f 
Signed-off-by: ahcorde <[email protected]>
@ahcorde
improved test
0b6ff27 
Signed-off-by: ahcorde <[email protected]>
@ahcorde
More time to test
158c918 
Signed-off-by: ahcorde <[email protected]>
@ahcorde
More time to test
f77cf37 
Signed-off-by: ahcorde <[email protected]>
@ahcorde
More time to test
3d4f07c 
Signed-off-by: ahcorde <[email protected]>
@ahcorde
another trial
875e49d 
Signed-off-by: ahcorde <[email protected]>
@ahcorde
added a debug trace
612eb49 
Signed-off-by: ahcorde <[email protected]>
@ahcorde
another attemp
ba99d4f 
Signed-off-by: ahcorde <[email protected]>
@ahcorde
Merge branch 'main' into ahcorde/6/markermanagerplugin
76bae05
chapulina
chapulina approved these changes Jul 20, 2021
View reviewed changes
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@chapulina chapulina merged commit 973cbcc into main Jul 20, 2021
@chapulina chapulina deleted the ahcorde/6/markermanagerplugin branch July 20, 2021 02:17
@chapulina chapulina mentioned this pull request Sep 8, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chapulina chapulina chapulina approved these changes

@jennuine jennuine Awaiting requested review from jennuine jennuine is a code owner

Assignees

@ahcorde ahcorde

Labels
🏯 fortress Ignition Fortress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ahcorde @chapulina